Skip to content

[RV64_DYNAREC] Support VMOVDQU opcode - #4224

Merged
ptitSeb merged 1 commit into
ptitSeb:mainfrom
zengdage:vmovdqu
Aug 11, 2026
Merged

[RV64_DYNAREC] Support VMOVDQU opcode#4224
ptitSeb merged 1 commit into
ptitSeb:mainfrom
zengdage:vmovdqu

Conversation

@zengdage

Copy link
Copy Markdown
Contributor

The VMOVDQU instruction is used by WeMeet for accelerating image processing. I test this pr by https://github.com/zengdage/x86_64-inst-test/blob/main/avx256/test_vmovdqu256.c

@ksco ksco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any specific reason to add the unaligned path?

@zengdage

Copy link
Copy Markdown
Contributor Author

Is there any specific reason to add the unaligned path?

I added that because MOVDQU handles unaligned paths, and legacy rv64 cores such as SiFive U74 lack hardware unaligned access support. Does box64’s rv64 backend plan to only target CPUs supporting hardware unalgined access?

@ptitSeb

ptitSeb commented Aug 10, 2026

Copy link
Copy Markdown
Owner

No, all CPU are targeted to be supported.

I think the question was more about: was this (unaligned) case needed, and is this (unaligned) branch tested?

@zengdage

Copy link
Copy Markdown
Contributor Author

No, all CPU are targeted to be supported.

I think the question was more about: was this case needed, and is this branch tested?

Sorry, I’m still confused and would appreciate some clarification.

My understanding of the IF_UNALIGNED(ip) macro is: By default, it assumes the CPU supports hardware unaligned access and takes the aligned‑path. If the CPU does not support it, an exception is triggered. The ip is recorded in a hash table, and the corresponding dynarec block is marked invalid. Code generation for this ip will take the unaligned‑path on the next emit time.

I notice that an unaligned‑path exists for MOVDQU Ex, Gx, but not for MOVDQU Gx, Ex. What is the difference between them? As I see it, when Ex is a memory operand in MOVDQU Gx, Ex, it can also be unaligned and should therefore require unaligned‑access handling. Could it be that sigbus_specialcases only handles store unaligned‑access cases, but not load unaligned‑access cases?

All risc-v CPUs I have available support unaligned access. So I changed IF_UNALIGNED(ip) to if (1) to force execution through the unaligned‑path for testing.

@ptitSeb

ptitSeb commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Yes, your understanding of the IF_UNALIGNED macro is correct (also, the block is forced as dirty so it will be regenerated at next run)

The process is: IF_UNALIGNED are always implemented in a "as-needed" process. You have a repeated SIGBUS that would benefit from IF_UNALIGNED, you implement it. Else, no need to clutter the code with IF_UNALIGNED cases.

All you hardware support unaligned access? You mean unaligned access to hardware mapped memory, like GPU memory? because the unaligned stuffs is for this case: unaligned access to memory mapped GPU memory (like on AMD gpu for example).

@zengdage

Copy link
Copy Markdown
Contributor Author

The process is: IF_UNALIGNED are always implemented in a "as-needed" process. You have a repeated SIGBUS that would benefit from IF_UNALIGNED, you implement it. Else, no need to clutter the code with IF_UNALIGNED cases.

Can I understand it this way?

  1. If VMOVDQU is only invoked a few times, there is no need to add IF_UNALIGNED. We can just take the SIGBUS trap and fall back to single‑byte access every time.
  2. If the VMOVDQU instruction gets called heavily, IF_UNALIGNED should be added to avoid frequent SIGBUS traps.

When I mentioned my cpus supporting unaligned accesses earlier, I was only considering normal cacheable memory. This does not cover device memory, atomic‑region memory, and other such memory types.

@ptitSeb

ptitSeb commented Aug 10, 2026

Copy link
Copy Markdown
Owner

The process is: IF_UNALIGNED are always implemented in a "as-needed" process. You have a repeated SIGBUS that would benefit from IF_UNALIGNED, you implement it. Else, no need to clutter the code with IF_UNALIGNED cases.

Can I understand it this way?

  1. If VMOVDQU is only invoked a few times, there is no need to add IF_UNALIGNED. We can just take the SIGBUS trap and fall back to single‑byte access every time.
  2. If the VMOVDQU instruction gets called heavily, IF_UNALIGNED should be added to avoid frequent SIGBUS traps.

When I mentioned my cpus supporting unaligned accesses earlier, I was only considering normal cacheable memory. This does not cover device memory, atomic‑region memory, and other such memory types.

Yes. The unaligned path is an optimisation. Too many SIGBUS will hurt the performances badly. But if there is no SIGBUS, why clutering the code with useless branch...

@zengdage

Copy link
Copy Markdown
Contributor Author

The process is: IF_UNALIGNED are always implemented in a "as-needed" process. You have a repeated SIGBUS that would benefit from IF_UNALIGNED, you implement it. Else, no need to clutter the code with IF_UNALIGNED cases.

Can I understand it this way?

  1. If VMOVDQU is only invoked a few times, there is no need to add IF_UNALIGNED. We can just take the SIGBUS trap and fall back to single‑byte access every time.
  2. If the VMOVDQU instruction gets called heavily, IF_UNALIGNED should be added to avoid frequent SIGBUS traps.

When I mentioned my cpus supporting unaligned accesses earlier, I was only considering normal cacheable memory. This does not cover device memory, atomic‑region memory, and other such memory types.

Yes. The unaligned path is an optimisation. Too many SIGBUS will hurt the performances badly. But if there is no SIGBUS, why clutering the code with useless branch...

It seems Box64 has made quite a few trade‑offs. I have removed the unaligned‑path. Please help review again. Thanks.

Comment thread src/dynarec/rv64/dynarec_rv64_avx_f3_0f.c Outdated
@ptitSeb
ptitSeb merged commit 83a9d67 into ptitSeb:main Aug 11, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants